[security] /v1/* 与 /api/admin/* 默认 fail-closed；新增首启自助初始化与全局未配置 Key 警告#109
Merged
james-6-23 merged 2 commits intoApr 30, 2026
Merged
Conversation
…n bootstrap
Previously the proxy fell back to fail-open behavior when no API key /
ADMIN_SECRET was configured: any unauthenticated client could hit /v1/*
and consume the entire account pool quota, or call /api/admin/* and
read / mutate sensitive data. At least 36 publicly deployed instances
have been observed to be vulnerable in their default configuration.
This commit changes the defaults:
- /v1/*: refuse with 503 when no public API key has been created.
An explicit opt-out (CODEX_ALLOW_ANONYMOUS=true) is provided for
internal / testing scenarios; the startup banner keeps warning while
it is active.
- /api/admin/*: refuse with 503 + code "bootstrap_required" when
ADMIN_SECRET is unset.
- New first-run bootstrap (admin/bootstrap.go):
GET /api/admin/bootstrap-status
POST /api/admin/bootstrap (only when uninitialized)
Hardening: sync.Mutex + double-check, fixed-window rate limit
(60s / 20 req), minimum 8-char secret, full SecurityAuditLog.
- Drop the previous "auto-generate ADMIN_SECRET" shortcut so users
always know which secret is in use.
- Default bind stays 0.0.0.0 to keep Docker / reverse-proxy deployments
working; CODEX_BIND lets users restrict to loopback.
- Frontend AuthGate gains a "First-run setup" screen with a confirm
field; a global SecurityBanner warns logged-in admins when no public
API key has been created.
- .env.example / .env.sqlite.example document the new switches.
Behavior change: existing deployments must (a) set an ADMIN_SECRET via
the new browser bootstrap page on first restart and (b) create at least
one public API key before /v1/* will start serving traffic again. Users
who consciously want the legacy unauthenticated behavior must opt in
with CODEX_ALLOW_ANONYMOUS=true.
📝 WalkthroughWalkthroughAdds a first-run admin bootstrap flow with two unauthenticated endpoints, enforces fail-closed admin/auth behavior, introduces bind-address and anonymous-v1 config, surfaces a startup security banner, updates proxy auth to block v1 when no keys exist unless allowed, and adds frontend bootstrap UI and security banner. Changes
Sequence DiagramssequenceDiagram
participant User as Browser User
participant Frontend as Frontend (AuthGate)
participant AdminAPI as Admin API (Bootstrap)
participant DB as Database
User->>Frontend: Load SPA
Frontend->>AdminAPI: GET /api/admin/bootstrap-status
AdminAPI->>DB: GetSystemSettings()
DB-->>AdminAPI: No AdminSecret
AdminAPI-->>Frontend: { needs_bootstrap: true }
Frontend->>User: Render bootstrap form
User->>Frontend: Submit admin secret
Frontend->>Frontend: Validate secret
Frontend->>AdminAPI: POST /api/admin/bootstrap {admin_secret}
AdminAPI->>AdminAPI: Rate-limit check & env check
AdminAPI->>DB: GetSystemSettings() (double-check)
DB-->>AdminAPI: No record
AdminAPI->>DB: UpdateSystemSettings(AdminSecret)
DB-->>AdminAPI: Success
AdminAPI-->>Frontend: 200 OK
Frontend->>Frontend: Persist admin key, set authenticated
Frontend->>User: Redirect to app
sequenceDiagram
participant Client as API Client
participant Proxy as Proxy (authMiddleware)
participant Config as Config (AllowAnonymousV1)
participant DB as Database (API keys)
Client->>Proxy: Request /v1/...
Proxy->>DB: hasAnyKeys()?
DB-->>Proxy: No keys
Proxy->>Config: Read AllowAnonymousV1
alt AllowAnonymousV1 = true
Proxy->>Client: Proceed (forward request)
else AllowAnonymousV1 = false
Proxy->>Proxy: Log audit V1_BLOCKED_NO_KEYS
Proxy-->>Client: 503 Service Unavailable (not configured)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 1/3 review remaining, refill in 25 minutes and 47 seconds. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/AuthGate.tsx (1)
79-90:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail closed on health-check failures instead of authenticating by default.
checkAuthcurrently treats unexpected statuses and fetch errors as authenticated (Line 86, Line 89), which is a fail-open path and conflicts with this PR’s security intent.Suggested patch
- if (res.status === 401) { + if (res.status === 401) { setAdminKey('') setStatus('need_login') } else if (res.status === 503) { setStatus('need_bootstrap') - } else { + } else if (res.ok) { setStatus('authenticated') + } else { + setStatus('need_login') } } catch { - setStatus('authenticated') + setStatus('need_login') }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/AuthGate.tsx` around lines 79 - 90, The health-check logic in checkAuth incorrectly treats non-401/503 responses and fetch errors as authenticated; change it to fail closed by only marking authenticated when the response is explicitly successful (e.g., res.status === 200), and for any other status or any caught exception call setStatus('need_login') (and clear credentials as appropriate with setAdminKey('')). Update the branch that currently sets status in the else and the catch block to enforce this strict check using the response status and exceptions, referencing the checkAuth function and the setStatus/setAdminKey calls to locate and modify the code.
🧹 Nitpick comments (1)
admin/bootstrap.go (1)
113-131: ⚡ Quick winAdd audit logs for 400 validation rejections on bootstrap endpoint.
ShouldBindJSON/empty/too-short/too-long branches return 400 withoutSecurityAuditLog, so unauthenticated probing attempts are not fully visible in security telemetry.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin/bootstrap.go` around lines 113 - 131, In the bootstrap handler in admin/bootstrap.go, add SecurityAuditLog calls before each 400 response in the ShouldBindJSON, empty secret, too-short (utf8.RuneCountInString(secret) < bootstrapMinSecret), and too-long (len(secret) > bootstrapMaxSecret) branches so validation rejections are recorded; include context such as the request remote IP (from c.Request.RemoteAddr or c.ClientIP()), the attempted AdminSecret masked or its length, the specific validation error string used in the c.JSON response, and the handler name (e.g., the current bootstrap handler) when invoking SecurityAuditLog; ensure you call the same SecurityAuditLog API used elsewhere in the codebase and keep behavior otherwise unchanged (still return the same 400 JSON responses afterward).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Around line 13-16: Update the ADMIN_SECRET description in .env.example to
reflect the actual bootstrap behavior: remove the "首次启动自动生成随机密钥" wording and
state that if ADMIN_SECRET is not set the server will be in fail-closed mode and
the first-time initialization must be completed via the /admin endpoint;
reference the bootstrap/fail-closed behavior so maintainers can correlate the
doc string with the initialization flow (look for ADMIN_SECRET and
bootstrap/fail-closed).
In @.env.sqlite.example:
- Around line 13-16: Update the ADMIN_SECRET comment in .env.sqlite.example to
reflect the current bootstrap behavior: replace the line that says the secret is
auto-generated on first start and written to DB with text stating that if
ADMIN_SECRET is not set the app will fail-closed during bootstrap and that the
admin account must be initialized via the /admin endpoint or by explicitly
setting the ADMIN_SECRET env var; ensure the comment references the ADMIN_SECRET
variable name so readers can find it easily.
In `@admin/bootstrap.go`:
- Around line 35-43: The rollover is race-prone: update bootstrapAllowRate to
perform a CAS-based window rollover on bootstrapState.windowStart (compare with
the loaded winStart and set to now using CompareAndSwap) so only one goroutine
resets the window; when the CAS succeeds reset bootstrapState.count to 0, and
otherwise proceed without resetting. After ensuring the correct windowStart,
atomically increment bootstrapState.count and compare against bootstrapMaxPerWin
(retry the increment if necessary or use the atomic Add result) to decide
allowance; reference bootstrapAllowRate, bootstrapState.windowStart,
bootstrapState.count, bootstrapWindowSec, and bootstrapMaxPerWin.
In `@config/config.go`:
- Line 73: 更新配置注释以与实际行为一致：在 config.go 中修正 BindAddress 的注释（以及同一文件中第 104-106
行相关注释）以反映当前默认值和行为（默认 0.0.0.0 且 ADMIN_SECRET 由 bootstrap 管理），确保提到需要显式设置为
127.0.0.1 或 0.0.0.0 时的含义和 bootstrap 管理自动 ADMIN_SECRET
的事实，并在注释中提供明确的安全提醒，便于未来维护者正确理解 BindAddress 与自动密钥生成的关系（定位标识符：BindAddress）。
In `@frontend/src/components/AuthGate.tsx`:
- Around line 147-173: Add a client-side max-length check in handleBootstrap to
mirror the backend validation: after trimming bsSecret and bsConfirm and before
setBsSubmitting(true), check if secret.length exceeds the backend max (use the
same MAX_SECRET_LEN value or import/define a matching constant) and call
setBsError(copy.errTooLong) then return; reference the handleBootstrap function,
bsSecret/bsConfirm, MIN_SECRET_LEN, setBsError and copy to locate where to
insert the check.
In `@frontend/src/components/SecurityBanner.tsx`:
- Around line 12-19: The banner text in SecurityBanner (the i18n key en.body and
corresponding body in Chinese) is absolute and incorrect when
CODEX_ALLOW_ANONYMOUS=true; update the copy to indicate the default behavior or
call out the anonymous-mode exception (e.g., prepend "By default," or append
"unless anonymous mode is enabled") so the message is accurate for both modes;
modify the strings referenced by body in the locale objects and ensure the CTA
and dismiss keys remain unchanged.
---
Outside diff comments:
In `@frontend/src/components/AuthGate.tsx`:
- Around line 79-90: The health-check logic in checkAuth incorrectly treats
non-401/503 responses and fetch errors as authenticated; change it to fail
closed by only marking authenticated when the response is explicitly successful
(e.g., res.status === 200), and for any other status or any caught exception
call setStatus('need_login') (and clear credentials as appropriate with
setAdminKey('')). Update the branch that currently sets status in the else and
the catch block to enforce this strict check using the response status and
exceptions, referencing the checkAuth function and the setStatus/setAdminKey
calls to locate and modify the code.
---
Nitpick comments:
In `@admin/bootstrap.go`:
- Around line 113-131: In the bootstrap handler in admin/bootstrap.go, add
SecurityAuditLog calls before each 400 response in the ShouldBindJSON, empty
secret, too-short (utf8.RuneCountInString(secret) < bootstrapMinSecret), and
too-long (len(secret) > bootstrapMaxSecret) branches so validation rejections
are recorded; include context such as the request remote IP (from
c.Request.RemoteAddr or c.ClientIP()), the attempted AdminSecret masked or its
length, the specific validation error string used in the c.JSON response, and
the handler name (e.g., the current bootstrap handler) when invoking
SecurityAuditLog; ensure you call the same SecurityAuditLog API used elsewhere
in the codebase and keep behavior otherwise unchanged (still return the same 400
JSON responses afterward).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b80473aa-b206-4727-8ed4-8efa780f7a5e
📒 Files selected for processing (10)
.env.example.env.sqlite.exampleadmin/bootstrap.goadmin/handler.goconfig/config.gofrontend/src/components/AuthGate.tsxfrontend/src/components/Layout.tsxfrontend/src/components/SecurityBanner.tsxmain.goproxy/handler.go
- config/config.go: align comments with current behaviour (default bind 0.0.0.0, security guarded by fail-closed + browser bootstrap, no auto-generated secret) - .env.example, .env.sqlite.example: rewrite ADMIN_SECRET notes to reflect the browser-based first-run bootstrap flow - admin/bootstrap.go: harden bootstrapAllowRate() with CAS-based window rollover so concurrent goroutines cannot reset the counter twice and exceed bootstrapMaxPerWin - frontend AuthGate: mirror backend max-length check (256 chars) to fail fast in the UI before sending the request - frontend SecurityBanner: soften absolute wording, mention CODEX_ALLOW_ANONYMOUS exception so the copy is consistent with the actual server behaviour
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/components/AuthGate.tsx (1)
126-148: 💤 Low valueInconsistent i18n: mixed
t()andcopyusage in login flow.The login handler and UI mix i18n approaches:
- Line 128 uses
t('auth.error')while lines 138/144 usecopy.loginError- Line 272 uses
t('settings.adminSecret')while line 278 usescopy.loginPlaceholderFor consistency (and to ensure the hardcoded COPY strings are actually used), consider replacing remaining
t()calls withcopy.*equivalents.♻️ Suggested fix
const handleLogin = async () => { if (!inputKey.trim()) { - setError(t('auth.error')) + setError(copy.loginError) return }For line 272, either add a
secretLabelkey to COPY or keep usingt()if this label should remain in the i18n system.Also applies to: 272-272
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/AuthGate.tsx` around lines 126 - 148, The code mixes i18n methods (t()) and the COPY object; update the login flow for consistency by replacing t('auth.error') in handleLogin with the appropriate COPY key (e.g., copy.loginError), and ensure the settings label t('settings.adminSecret') and the input placeholder use COPY (e.g., copy.secretLabel and copy.loginPlaceholder); if those COPY keys do not exist, add them to the COPY object and then reference copy.secretLabel/copy.loginPlaceholder in the component, keeping all uses in handleLogin (setError, catch block), setAdminKey, setStatus, inputKey, and UI labels consistent with COPY.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@admin/bootstrap.go`:
- Around line 98-106: The doc comment above PostBootstrap currently says the
minimum secret length is 12 runes but the code defines bootstrapMinSecret = 8;
update the comment to reflect the actual constraint (8 runes) so the
documentation matches the constant, e.g. change "校验最小长度（12 个 rune）" to "校验最小长度（8
个 rune）"; verify references to PostBootstrap and bootstrapMinSecret in the
comment are consistent after the edit.
---
Nitpick comments:
In `@frontend/src/components/AuthGate.tsx`:
- Around line 126-148: The code mixes i18n methods (t()) and the COPY object;
update the login flow for consistency by replacing t('auth.error') in
handleLogin with the appropriate COPY key (e.g., copy.loginError), and ensure
the settings label t('settings.adminSecret') and the input placeholder use COPY
(e.g., copy.secretLabel and copy.loginPlaceholder); if those COPY keys do not
exist, add them to the COPY object and then reference
copy.secretLabel/copy.loginPlaceholder in the component, keeping all uses in
handleLogin (setError, catch block), setAdminKey, setStatus, inputKey, and UI
labels consistent with COPY.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: daac5f8b-666a-418d-9770-5463e70aa662
📒 Files selected for processing (6)
.env.example.env.sqlite.exampleadmin/bootstrap.goconfig/config.gofrontend/src/components/AuthGate.tsxfrontend/src/components/SecurityBanner.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- .env.sqlite.example
- .env.example
Comment on lines
+98
to
+106
| // PostBootstrap 接收用户在浏览器中输入的初始管理密钥并写入数据库。 | ||
| // | ||
| // 安全约束： | ||
| // 1. 仅在系统未配置 ADMIN_SECRET 时可用，否则一律 409； | ||
| // 2. 通过互斥锁 + 双重检查避免并发写入； | ||
| // 3. 简单全局限频，防止扫描器穷举； | ||
| // 4. 校验最小长度（12 个 rune），避免过弱密钥； | ||
| // 5. 全程审计日志。 | ||
| func (h *Handler) PostBootstrap(c *gin.Context) { |
There was a problem hiding this comment.
Comment says 12 runes but constant is 8.
The doc comment on line 104 states "校验最小长度（12 个 rune）" but bootstrapMinSecret = 8 on line 31. Update the comment to match the actual constraint.
📝 Suggested fix
// 安全约束：
// 1. 仅在系统未配置 ADMIN_SECRET 时可用，否则一律 409；
// 2. 通过互斥锁 + 双重检查避免并发写入；
// 3. 简单全局限频，防止扫描器穷举；
-// 4. 校验最小长度（12 个 rune），避免过弱密钥；
+// 4. 校验最小长度（8 个 rune），避免过弱密钥；
// 5. 全程审计日志。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@admin/bootstrap.go` around lines 98 - 106, The doc comment above
PostBootstrap currently says the minimum secret length is 12 runes but the code
defines bootstrapMinSecret = 8; update the comment to reflect the actual
constraint (8 runes) so the documentation matches the constant, e.g. change
"校验最小长度（12 个 rune）" to "校验最小长度（8 个 rune）"; verify references to PostBootstrap
and bootstrapMinSecret in the comment are consistent after the edit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
漏洞概要 / TL;DR
当前
main分支（含已发布的v2.0.0）默认存在两条未授权直通通道：/v1/*在未创建任何对外 API Key 时不做鉴权：任何能访问端口的人都可以直接调用
/v1/chat/completions、/v1/responses、/v1/messages、/v1/images/generations等接口，消耗账号池中所有 Codex 账号的配额。/api/admin/*在未设置ADMIN_SECRET时同样无鉴权放行：攻击者可以读取/写入账号、删除 API Key、改设置、触发 OAuth 流程等。
相关代码（修复前）：
proxy/handler.go: authMiddleware：if !h.hasAnyKeys() { c.Next(); return }—— fail-openadmin/handler.go: adminAuthMiddleware：if adminSecret == "" { c.Next(); return }—— fail-open影响范围
/v1/*全部、/api/admin/*全部修复方案（fail-closed + first-run bootstrap）
1.
/v1/*——proxy/handler.go503 service_unavailable，并写入安全审计日志
V1_BLOCKED_NO_KEYS。CODEX_ALLOW_ANONYMOUS=true仅供内网 / 测试场景，打开后启动 banner 会持续告警。
2.
/api/admin/*——admin/handler.goADMIN_SECRET时返回503 + code: bootstrap_required，并写入审计日志
ADMIN_BLOCKED_NO_SECRET。避免出现"用户不知道但其实已存在"的默认密钥（这种自动密钥本身也是攻击面）。
3. 首启自助初始化 ——
admin/bootstrap.go（新增）两个无鉴权端点（注册在 admin 鉴权中间件之外）：
GET /api/admin/bootstrap-status→{needs_bootstrap, source: env|database|empty}POST /api/admin/bootstrap→ 仅当系统未初始化时接受一次写入，约束包含：sync.Mutex+ 进入临界区的 二次检查（杜绝并发写入）409SecurityAuditLog4. 前端 ——
frontend/src/components/AuthGate.tsx、SecurityBanner.tsx（新增）AuthGate启动时先调bootstrap-status：needs_bootstrap=true→ 显示「首次使用：设置管理密钥」表单（含二次输入校验、最少 8 位）SecurityBanner：登录后周期性查询 API Key 数量，为 0 时显示红色横幅提示「尚未配置对外 API Key」并提供跳转入口；用户可以「我知道了」
暂时关闭 24h。
5. 监听地址 / 文档
0.0.0.0以兼容 Dockerports:、反向代理与生产部署 ——曾在迭代中尝试默认
127.0.0.1，但会破坏所有 docker-compose 部署，最终回退为：
0.0.0.0+ 启动 banner 提示需配防火墙 + HTTPSCODEX_BIND=127.0.0.1给希望严格回环的用户.env.example/.env.sqlite.example增补CODEX_BIND、CODEX_ALLOW_ANONYMOUS、ADMIN_SECRET三个开关的说明与最佳实践行为变化（破坏性，请阅读）
ADMIN_SECRET/v1/*仍可被任意调用消耗配额/v1/*返回 503，提示需创建 KeyCODEX_ALLOW_ANONYMOUS=true后启动（banner 会持续告警）0.0.0.0，端口映射可用0.0.0.0）升级路径：
# 已部署的实例： 1. 拉取新镜像 / 二进制并重启 2. 浏览器访问 /admin/，按提示设置一个强随机 ADMIN_SECRET（≥8 位） 3. 在「API 密钥」页面创建至少一把对外 Key 后再恢复对外提供服务测试矩阵（手工验证全部通过）
GET /api/admin/bootstrap-statusneeds_bootstrap:true503+code: bootstrap_required400拒绝200 ok:true409拒绝/v1/models503 service_unavailableCODEX_ALLOW_ANONYMOUS=true后调/v1/models/api/admin/accounts2000.0.0.0端口映射CODEX_BIND=127.0.0.1时局域网 IP 访问Connection refused+lsof显示127.0.0.1:8081文件清单
新增：
admin/bootstrap.gofrontend/src/components/SecurityBanner.tsx修改：
proxy/handler.go—/v1/*fail-closedadmin/handler.go—/api/admin/*fail-closed + 注册 bootstrap 路由config/config.go— 新增BindAddress、AllowAnonymousV1main.go— 启动安全自检 banner（不再自动生成密钥）frontend/src/components/AuthGate.tsx— 新增need_bootstrap状态与初始化页frontend/src/components/Layout.tsx— 注入全局SecurityBanner.env.example、.env.sqlite.example— 文档化三个安全相关开关致维护者
强烈建议：
v2.0.1），并在 release notes中明确列出「安全修复 + 行为变化」。
docs/SECURITY.md，告知未来漏洞披露渠道。Summary by CodeRabbit
New Features
Behavior Changes
Documentation